fix: include parent span IDs in daemon merge writes - #52
Merged
Merged
Conversation
max-braintrust
changed the title
max-braintrust
force-pushed
the
fix/codex-parent-identity
branch
from
September 4, 2026 23:46
12b9682 to
a7d40f2
Compare
max-braintrust
changed the title
Collaborator
|
max-braintrust Looks like there are a bunch of conflicts. Can you rebase this? |
Andrew Kent (realark)
approved these changes
Sep 22, 2026
max-braintrust
force-pushed
the
fix/codex-parent-identity
branch
2 times, most recently
from
September 22, 2026 20:03
571936b to
11300ce
Compare
Closes three gaps left by the previous commit, each covered by a test that fails without the fix: - opencode: the `session.idle` root merge dropped the parent. Idle runs after every turn; only the sibling close-root arm had been converted. - codex: `update_open_main_root_source` dropped the external parent. Every main-scope Stop closes the root handle, so a later SessionStart (resume/compact) merges statelessly in the same live session. - grok: the translator had been skipped entirely. All 13 merge sites now carry parents, with the owning span id recorded on OpenLlm/OpenTool/CompletedTool and the attached parent on the session row. Two of these carry a late_merge_key, meaning the merge is expected after the terminal row. Coverage: - Wire codex's reduce() and grok's whole suite to the invariant. Grok is the only suite without a reduce() choke point, so merges are checked per batch against an IdentityLedger that remembers inserts across hooks. - Grok's suite never built a SessionCtx with a config, so the attached-parent path was untested; add a test that asserts every session-row merge repeats the external parent. Without the grok fix, 16 of 19 grok tests now fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
max-braintrust
force-pushed
the
fix/codex-parent-identity
branch
from
September 22, 2026 21:04
11300ce to
36bc8be
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Merge writes from the plugins did not include parent ids. This was a problem if no successful insert write proceeded the merge write for a particular span, because the span would be attached to the trace without a parent. Spans without parents automatically get is_root=true, so this can lead to a situation where a trace has multiple spans marked as the root.
Solution
Include the correct root_span_id and parent span IDs in every merge write, using the same deterministic hierarchy used by the corresponding insert. This allows a merge to preserve the span’s hierarchy even when the initial insert was not persisted, without requiring a sink-side identity cache.
Add regression coverage to verify that: